Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding basic dynamic mode #42

Merged
merged 20 commits into from
Nov 8, 2024
Merged

Adding basic dynamic mode #42

merged 20 commits into from
Nov 8, 2024

Conversation

SebaLukas
Copy link
Collaborator

Describe your changes

This PR adds basic support for dynamic mode. Tested on the Testival in France and there was no problems.

Issue ticket number and link

Support for dynamic mode was missing

Checklist before requesting a review

  • I have performed a self-review of my code
  • I have made corresponding changes to the documentation
  • I read the contribution documentation and made sure that my changes meet its requirements

…rialization ScheduleExchange messages tests

Signed-off-by: Sebastian Lukas <[email protected]>
…arge_dynamic_mode to feedback

Signed-off-by: Sebastian Lukas <[email protected]>
…mobilityneeds (1 and 2) modes in the EvseSetupConfig struct

Signed-off-by: Sebastian Lukas <[email protected]>
…needs_mode support to schedule_exchange & dc_charge_loop, adding dc_charge_loop states test

Signed-off-by: Sebastian Lukas <[email protected]>
Copy link
Contributor

@a-w50 a-w50 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really like the consistent use of test cases. I would still ask to refactor the code a bit to avoid code duplication and make it more readable.

static constexpr auto SCHEDULED_POWER_DURATION_S = 86400;

Header header;
ResponseCode response_code;

ScheduleExchangeResponse() :
processing(Processing::Finished), control_mode(std::in_place_type<Dynamic_SEResControlMode>){};
processing(Processing::Finished), control_mode(std::in_place_type<Dynamic_SEResControlMode>) {};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should use default values for the member variables right away, not defining custom constructors

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have no preference. So I can change this in a separate PR for all message definitions.

@a-w50 a-w50 self-requested a review November 8, 2024 12:39
@SebaLukas SebaLukas merged commit a6eebd3 into main Nov 8, 2024
3 of 4 checks passed
@SebaLukas SebaLukas deleted the feat/adding-basic-dynamic-mode branch November 8, 2024 14:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants